Add support for Quartz clustered jobs #5529
Conversation
machi1990
force-pushed
the
3520
branch
2 times, most recently
from
b6050b6 to
58bdb98
Compare
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzScheduler.java
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzScheduler.java
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzBuildTimeConfig.java
Outdated
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzBuildTimeConfig.java
Outdated
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzBuildTimeConfig.java
Outdated
Show resolved
Hide resolved
|
Test failure related. I am still taking into account the feedbacks. |
machi1990
force-pushed
the
3520
branch
3 times, most recently
from
2508516 to
2487eee
Compare
...rtz/runtime/src/main/java/io/quarkus/quartz/runtime/QuarkusQuartzConnectionPoolProvider.java
Outdated
Show resolved
Hide resolved
| this.dataSource = instanceHandle.get(); | ||
| } else { | ||
| String message = String.format( | ||
| "JDBC Store configured but '%s' datasource is missing. You can configure your datasource by following the guide available at: https://quarkus.io/guides/datasource-guide", |
There was a problem hiding this comment.
| "JDBC Store configured but '%s' datasource is missing. You can configure your datasource by following the guide available at: https://quarkus.io/guides/datasource-guide", | |
| "JDBC Store configured but '%s' datasource is missing. You can configure your datasource by following the guide available at: https://quarkus.io/guides/datasource", |
extensions/quartz/deployment/pom.xml
Outdated
| <artifactId>quarkus-quartz-deployment</artifactId> | ||
| <name>Quarkus - Scheduler Quartz - Deployment</name> | ||
| <artifactId>quarkus-quartz-deployment</artifactId> | ||
| <name>Quarkus - Scheduler Quartz - Deployment</name> |
There was a problem hiding this comment.
Either "Quartz Scheduler" or "Scheduler - Quartz" or just "Quartz" but the current order looks weird.
I suppose if we judge that quarkus-quartz is good enough, it should be Quartz. Then we can detail in the description and the metadata of the extension.
There was a problem hiding this comment.
Well, "Scheduler" was there for a good reason - it's a link to the Scheduler extension this Quartz extension is build on (and basically extends it's functionality).
There was a problem hiding this comment.
I can see that but it's hard to grasp IMHO. Maybe something like Scheduler - Quartz might be better.
And in this case, why is the extension named quarkus-quartz and not quarkus-scheduler-quartz? Not saying I want the latter, just asking if we should be consistent somehow.
extensions/quartz/deployment/src/main/java/io/quarkus/quartz/deployment/QuartzProcessor.java
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzRunTimeConfig.java
Outdated
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzScheduler.java
Outdated
Show resolved
Hide resolved
extensions/quartz/runtime/src/main/java/io/quarkus/quartz/runtime/QuartzScheduler.java
Outdated
Show resolved
Hide resolved
gsmet
changed the title
machi1990
force-pushed
the
3520
branch
6 times, most recently
from
55cea06 to
a7a283a
Compare
docs/src/main/asciidoc/index.adoc
Outdated
| @@ -20,6 +20,7 @@ include::quarkus-intro.adoc[tag=intro] | |||
| * link:lifecycle.html[Application Initialization and Termination] | |||
| * link:rest-json.html[Writing JSON REST Services] | |||
| * link:scheduler.html[Schedule Periodic Tasks] | |||
| * link:quartz.html[Scheduler Periodic Clustered Tasks] | |||
There was a problem hiding this comment.
| * link:quartz.html[Scheduler Periodic Clustered Tasks] | |
| * link:quartz.html[Schedule Periodic Clustered Tasks] |
There was a problem hiding this comment.
Schedule Periodic Tasks with Quartz ?
docs/src/main/asciidoc/quartz.adoc
Outdated
| * an IDE | ||
| * JDK 1.8+ installed with `JAVA_HOME` configured appropriately | ||
| * Apache Maven 3.5.3+ | ||
| * Docker and docker compose installed on your machine |
There was a problem hiding this comment.
| * Docker and docker compose installed on your machine | |
| * Docker and Docker Compose installed on your machine |
docs/src/main/asciidoc/quartz.adoc
Outdated
| include::./attributes.adoc[] | ||
|
|
||
| Modern applications often need to run specific tasks periodically. | ||
| In this guide, you learn how to schedule periodic clustered tasks using the http://www.quartz-scheduler.org/[Quartz extension]. |
There was a problem hiding this comment.
| In this guide, you learn how to schedule periodic clustered tasks using the http://www.quartz-scheduler.org/[Quartz extension]. | |
| In this guide, you learn how to schedule periodic clustered tasks using the http://www.quartz-scheduler.org/[Quartz] extension. |
docs/src/main/asciidoc/quartz.adoc
Outdated
|
|
||
| == Architecture | ||
|
|
||
| In this guide, we create a straightforward application accessible using HTTP to get the current list of all created tasks by schedulers running on two instances of the same application. |
There was a problem hiding this comment.
This sentence is a bit confusing IMHO.
There was a problem hiding this comment.
Thanks for pointing it out. Let me revisit it.
docs/src/main/asciidoc/quartz.adoc
Outdated
| * an `org.acme.quartz.TaskResource` resource | ||
| * an associated test | ||
|
|
||
| The Maven project also imports the Quarkus quartz extension. |
There was a problem hiding this comment.
| The Maven project also imports the Quarkus quartz extension. | |
| The Maven project also imports the Quarkus Quartz extension. |
docs/src/main/asciidoc/quartz.adoc
Outdated
| == Run the application in Dev Mode | ||
|
|
||
| Run the application with: `./mvnw quarkus:dev`. | ||
| After a few seconds, open another terminal and run `curl localhost:8080/tasks` to verify that we have task created. |
There was a problem hiding this comment.
we have task created doesn't look correct.
| As usual, the application can be packaged using `./mvnw clean package` and executed using the `-runner.jar` file. | ||
| You can also generate the native executable with `./mvnw clean package -Pnative`. | ||
|
|
||
| == Packaging the application and run several instances |
docs/src/main/asciidoc/quartz.adoc
Outdated
| You can also generate the native executable with `./mvnw clean package -Pnative`. | ||
|
|
||
| == Packaging the application and run several instances | ||
| The application can be packaged using `./mvnw clean package`. Once the build is successfully, run the below command |
There was a problem hiding this comment.
| The application can be packaged using `./mvnw clean package`. Once the build is successfully, run the below command | |
| The application can be packaged using `./mvnw clean package`. Once the build is successful, run the below command: |
docs/src/main/asciidoc/quartz.adoc
Outdated
| and pull requests should be submitted there: | ||
| https://github.com/quarkusio/quarkus/tree/master/docs/src/main/asciidoc | ||
| //// | ||
| = Quarkus - Scheduling Periodic Clustered Tasks |
There was a problem hiding this comment.
It will be a guide about this only? Won't we expose more Quartz features?
There was a problem hiding this comment.
Should it be renamed to Quarkus - Scheduling Periodic Tasks with Quartz ?
There was a problem hiding this comment.
AFAIK we don't expose any Quarzt APIs at the moment and I think it's reasonable to keep it this way because we'd like to encourage users to continue using our scheduler extension API. Therefore it's ok to name the guide like this.
There was a problem hiding this comment.
@mkouba Yes that makes sense. I have already renamed it in the way I proposed above. I am also thinking of adding a link from the current scheduler extension to this guide with something like - use the quartz extension to schedule clustered jobs
WDYT?
There was a problem hiding this comment.
I have already renamed it in the way I proposed above...
Well, my comment was rather that it's ok to use "clustered" in the title because that's what this extension is for, ie. the message should be something like "if you really need clustered scheduler use the quartz extension".
I am also thinking of adding a link from the current scheduler extension to this guide with something like...
I like it! Both guides should probably reference each other?
There was a problem hiding this comment.
Great. I’ll add it later on, thanks.
There was a problem hiding this comment.
I have added the links to both guides.
There was a problem hiding this comment.
OK, sorry, I thought the whole point of this extension was to expose the Quartz APIs at some point.
machi1990
force-pushed
the
3520
branch
5 times, most recently
from
2690d3f to
730f4fb
Compare
| @@ -17,6 +17,8 @@ API or configuration properties might change as the extension matures. | |||
| Feedback is welcome on our https://groups.google.com/d/forum/quarkus-dev[mailing list] or as issues in our https://github.com/quarkusio/quarkus/issues[GitHub issue tracker]. | |||
| ==== | |||
|
|
|||
| TIP: If you really need clustered scheduler use the link:quartz[Quartz] extension | |||
There was a problem hiding this comment.
| TIP: If you really need clustered scheduler use the link:quartz[Quartz] extension | |
| TIP: If you need a clustered scheduler use the link:quartz[Quartz] extension |
|
There is a conflict apparently in the doc. I also added one last comment. Otheriwise it looks mostly good to me. @mkouba so as for the artifact names, should we go with |
I don't have a preference. When I created the quartz extension I used |
@gsmet thanks for the heads up about the conflict. It should be fixed now. I kept the name as
I kept |
|
I force pushed two very minor adjustments in the doc and extension metadata. Let's wait for CI and merge. Thanks! |
gsmet
added
the
triage/waiting-for-ci
|
Once this goes in, we can give some love to the quickstart PR - quarkusio/quarkus-quickstarts#389 :-) |
Follows up #3783
Fixes #3520
Based on #5481, the reason behind opening as draft PR./cc @mkouba
Quartz Quickstart PR opened quarkusio/quarkus-quickstarts#389